Skip to content

Conversation

@melitele
Copy link
Contributor

@melitele melitele commented Nov 3, 2025

Launch Checklist

Implements #6495

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Add an entry to CHANGELOG.md under the ## main section.

@HarelM
Copy link
Collaborator

HarelM commented Nov 3, 2025

Is this considered a style spec change?
Cc: @louwers

@louwers
Copy link
Member

louwers commented Nov 3, 2025

Native doesn't support this yet so as far as Native is concerned changes are not a problem.

@HarelM
Copy link
Collaborator

HarelM commented Nov 4, 2025

There should probably a design proposal issue instead of this PR to discuss this, but as far as I understand this change, it might mean that every feature can change the visibility of a layer?
If so, it's problematic.
This is not a minor change to the spec to change how visibility is behaving, so I think a proper discussion (design proposal issue) is needed here.

@melitele
Copy link
Contributor Author

melitele commented Nov 4, 2025

... every feature can change the visibility of a layer?

Of course not, this is what filters are for.

This PR makes visibility dependent only on global state as per #6495

@hiddewie
Copy link
Contributor

hiddewie commented Nov 8, 2025

I wrote a design proposal in #1364

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2025

Codecov Report

❌ Patch coverage is 23.18841% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.59%. Comparing base (c55de75) to head (9778bc2).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/expression/visibility.test.js 0.00% 52 Missing ⚠️
src/expression/visibility.ts 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1359      +/-   ##
==========================================
- Coverage   95.37%   92.59%   -2.78%     
==========================================
  Files         114      113       -1     
  Lines        7395     4458    -2937     
  Branches     2326     1394     -932     
==========================================
- Hits         7053     4128    -2925     
+ Misses        342      330      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

add tests to show that `zoom`, `feature` and `feature-state` are ignored
@melitele melitele force-pushed the global-state-visibility branch from 4f83103 to 9778bc2 Compare November 9, 2025 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants